8353749: Improve security warning when using JKS or JCEKS keystores #27624

haimaychao · 2025-10-03T16:26:13Z

This PR improves security warning when using JKS or JCEKS keystores.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8353749: Improve security warning when using JKS or JCEKS keystores (Enhancement - P3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27624/head:pull/27624
$ git checkout pull/27624

Update a local copy of the PR:
$ git checkout pull/27624
$ git pull https://git.openjdk.org/jdk.git pull/27624/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27624

View PR using the GUI difftool:
$ git pr show -t 27624

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27624.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-10-03T16:27:11Z

👋 Welcome back hchao! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-10-03T16:27:32Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-10-03T16:28:12Z

@haimaychao The following label will be automatically applied to this pull request:

security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-10-03T16:31:30Z

Webrevs

01: Full - Incremental (452e5cda)
00: Full (560d4163)

seanjmullan · 2025-10-06T12:24:34Z

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties

 jar.contains.internal.inconsistencies.result.in.different.contents.via.jarfile.and.jarinputstream=This JAR file contains internal inconsistencies that may result in different contents when reading via JarFile and JarInputStream:
 signature.verification.failed.on.entry.1.when.reading.via.jarinputstream=Signature verification failed on entry %s when reading via JarInputStream
 signature.verification.failed.on.entry.1.when.reading.via.jarfile=Signature verification failed on entry %s when reading via JarFile
+outdated.storetype.warning=%1$s uses outdated cryptographic algorithms and will be removed in a future release. Migrate to PKCS12 using:\n\


Call this "jks.storetype.warning" so it is consistent with keytool.properties.

seanjmullan · 2025-10-06T12:27:08Z

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

    private boolean signerSelfSigned = false;
    private boolean allAliasesFound = true;
    private boolean hasMultipleManifests = false;
+    private boolean outdatedFormat = false;


Suggest calling this variable "weakKeyStore".

seanjmullan · 2025-10-06T12:32:29Z

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

+            File storeFile = new File(keyStoreName);
+            if (storeFile.isFile()) {
+                KeyStore keyStore = KeyStore.getInstance(storeFile, storepass);
+                realStoreType = keyStore.getType();
+                if (realStoreType.equalsIgnoreCase("JKS")
+                        || realStoreType.equalsIgnoreCase("JCEKS")) {
+                    outdatedFormat = true;
+                }
+            }


I don't think you need the realStoreType field. If you move this check to the end of the else block starting on line 2424 (which means the keystore is a file), and just check the KeyStore.type() I think it should be sufficient, ex:

if (store.getType().equalsIgnoreCase("JKS") || store.getType().equalsIgnoreCase("JCEKS")) { weakKeyStore = true; }

seanjmullan · 2025-10-06T12:33:11Z

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

+        if (outdatedFormat) {
+            warnings.add(String.format(rb.getString(
+                    "outdated.storetype.warning"),
+                    realStoreType, keystore));


You can pass store.getType() instead of realStoreType here.

seanjmullan · 2025-10-06T12:35:32Z

src/java.base/share/classes/java/security/KeyStore.java

        return (provider == null) ? "(no provider)" : provider.getName();
    }

+    private static void outdatedKeyStoreLog(String type) {


I think it would be simpler to include this warning in the constructor of sun.security.provider.JavaKeyStore. Then you don't need to call this method.

Moved the warning to engineLoad() in JceKeyStore and JavaKeyStore, instead of in their constructors. Otherwise, we may get false positive warnings from KeyStore.getInstance() when it goes thru the list of providers to probe for the right keystore.

seanjmullan · 2025-10-10T13:40:54Z

test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java

 /*
 * @test
- * @bug 8217375 8260286 8267319
+ * @bug 8217375 8260286 8267319 8353749


This is not specifically testing this issue, so I don't think you should include the bugid.

seanjmullan · 2025-10-10T13:46:55Z

test/jdk/sun/security/tools/jarsigner/warnings/Test.java

I think you should create a new test which is a subclass of this Test which checks that JKS and JCEKS produce the proper warnings when using jarsigner.

seanjmullan · 2025-10-10T14:19:18Z

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java

                            is.close();
                        }
                    }
+                    if (store.getType().equalsIgnoreCase("JKS")


We should put in the same logic here as in keytool to check if the real storetype is JKS or JCEKS. See

jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java

Line 1392 in f6d77cb

// Probe for real type. A JKS can be loaded as PKCS12 because

wangweij · 2025-10-10T15:31:09Z

Do we need to add a warning at store? For example, someone getInstance("JKS") and load(null, null), and finally store it.

haimaychao added 2 commits September 30, 2025 23:12

8353749: Improve security warning when using JKS or JCEKS keystores

c2b90c0

Merge

560d416

openjdk bot added the security [email protected] label Oct 3, 2025

openjdk bot added the rfr Pull request is ready for review label Oct 3, 2025

seanjmullan reviewed Oct 6, 2025

View reviewed changes

haimaychao added 4 commits October 9, 2025 23:38

Remove test TestOutdatedKeyStore.java

9fc01d1

Revert changes to KeyStore.java

3a2334f

Updates with review comments

8246e4d

Updates with review comments

452e5cd

seanjmullan reviewed Oct 10, 2025

View reviewed changes

8353749: Improve security warning when using JKS or JCEKS keystores #27624

Are you sure you want to change the base?

8353749: Improve security warning when using JKS or JCEKS keystores #27624

Uh oh!

Conversation

haimaychao commented Oct 3, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Oct 3, 2025

Uh oh!

openjdk bot commented Oct 3, 2025

Uh oh!

openjdk bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangweij commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

haimaychao commented Oct 3, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 3, 2025 •

edited

Loading

mlbridge bot commented Oct 3, 2025 •

edited

Loading